Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TVM deploy #1326

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add TVM deploy #1326

wants to merge 7 commits into from

Conversation

liueo
Copy link

@liueo liueo commented May 28, 2020

Provide a demo for using pretrained GluonCV models in C++ environments with the support of TVM.

@mli
Copy link
Member

mli commented May 28, 2020

Job PR-1326-1 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-1326/1/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

This is a demo application which illustrates how to use pretrained GluonCV models in c++ environments with the support of TVM.

## Build instruction
Always use `git clone --recursive`, if not, we can update tvm submodule `git submodule update --recursive --init`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike a separate project, adding tvm into gluoncv as a submodule is an overkill.
Let's remove the submodule of tvm and add one setence to ask user to git clone the tvm module

Please clone the full TVM repository by `git clone --recursive https://github.com/apache/incubator-tvm.git`

@@ -0,0 +1,275 @@
"""Helper utils for export HybridBlock to symbols."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

export.py, export_classify.py, export_detection.py are duplicates?

Copy link
Member

@xinyu-intel xinyu-intel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just suggestions:)

.gitmodules Outdated
@@ -0,0 +1 @@

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty line

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@@ -0,0 +1,4 @@
build/*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove .git files under this dir


TVMArrayCopyFromBytes(input,image.data(),224*224*3*4);

tvm::runtime::PackedFunc set_input = mod.GetFunction("set_input");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can use zero_copy?

else:
data_shape = (3, 512, 605)
model = gcv.model_zoo.get_model(args.model, pretrained=True)
export_tvm("model", model, data_shape, ctx=mx.cpu(), preprocess=TvmPreprocess(), target='llvm -mcpu=core-avx2', use_autotvm=False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add args for ctx and target?

Copy link
Author

@liueo liueo May 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, those variables can be set as arguments and assigned values according to specific environment. I just provide a demo for deployment using TVM, so feel free to make changes when needed.

@mli
Copy link
Member

mli commented May 29, 2020

Job PR-1326-3 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-1326/3/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

@liueo liueo requested a review from zhreshold May 30, 2020 03:06
@bryanyzhu
Copy link
Collaborator

Just curious, I saw two "clipp.hpp" files, is it possible to reuse it instead of putting it in two folders? Thank you.

@zhreshold
Copy link
Member

@liueo See other reviewer's comments, please try to reduce duplicate code

@mli
Copy link
Member

mli commented Jun 2, 2020

Job PR-1326-4 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-1326/4/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

@zhreshold
Copy link
Member

@liueo Can you address the comments? We could merge this once done ASAP

INCLUDE_DIRECTORIES(${HOME_TVM}/3rdparty/dmlc-core/include)
INCLUDE_DIRECTORIES(${HOME_TVM}/3rdparty/dlpack/include)

set(PNG_LIBS libpng.a)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find libpng, libz, libjpeg anywhere in the PR so this much be something hardcoded.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we want to build libjpeg, libpng statically and link to them, these libraries should be prepared before building the full project. I didn't provide these files because they depend on specific operating system. I write some comments about the procedure in README and thought developers would build these libraries according to their target platform. Could you please give me some suggestions on how to make changes? Thanks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ideal way is to provide the build system for libjpeg and libpng in a cross-platform way(cmake).
We can include the source of libpng/jpeg in this case. Build for static linking, and finally include them in this demo probject CMakeList.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is using CMake, Why not use find_package() for these?

@mli
Copy link
Member

mli commented Aug 7, 2020

Job PR-1326-8 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-1326/8/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

@mli
Copy link
Member

mli commented Aug 24, 2020

Job PR-1326-9 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-1326/9/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

@mli
Copy link
Member

mli commented Sep 7, 2020

Job PR-1326-10 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-1326/10/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

@mli
Copy link
Member

mli commented Sep 21, 2020

Job PR-1326-11 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-1326/11/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

@mli
Copy link
Member

mli commented Oct 5, 2020

Job PR-1326-12 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-1326/12/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

@mli
Copy link
Member

mli commented Oct 19, 2020

Job PR-1326-13 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-1326/13/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

@mli
Copy link
Member

mli commented Nov 19, 2020

Job PR-1326-14 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-1326/14/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

@mli
Copy link
Member

mli commented Dec 3, 2020

Job PR-1326-15 is done.
Docs are uploaded to http://gluon-vision-staging.s3-website-us-west-2.amazonaws.com/PR-1326/15/index.html
Code coverage of this PR: pr.svg vs. Master: master.svg

Copy link

@harshsingh32 harshsingh32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants